Extract syncing protocol from `sc-network` #12828

altonen · 2022-12-02T09:55:24Z

This PR extracts the syncing protocol from sc-network by introducing a SyncingEngine object which holds all the syncing-related code that was previously in Protocol. It's an async task that can be communicated with using SyncingService. SyncingEngine also owns ChainSync and is responsible for polling it.

It was discovered in paritytech/polkadot-sdk#556 that the current handshaking implementation is incorrect and requires quite a bit of work to get it working properly. I would rather not tinker with the support beams in this PR and would like to address the handshaking issue in a follow-up PR if it's alright.

The only new code this PR adds is the SyncingService which models the already-existing NetworkService concept, meaning it now implements the traits (such as NetworkBlock and JustificatonSyncLink) that were previously implemented by NetworkService. Other than that, this PR is just moving code away from sc-network and fixing any references other crates made to syncing-related concepts. The syncing code is in dire need of refactoring but that can be addressed in future PRs.

Resolves #8686

polkadot companion: paritytech/polkadot#6380
cumulus companion: paritytech/cumulus#1939

Add supplementary asynchronous API for the import queue which means it can be run as an independent task and communicated with through the `ImportQueueService`. This commit removes removes block and justification imports from `sc-network` and provides `ChainSync` with a handle to import queue so it can import blocks and justifications. Polling of the import queue is moved complete out of `sc-network` and `sc_consensus::Link` is implemented for `ChainSyncInterfaceHandled` so the import queue can still influence the syncing process.

Some of the tests have to be rewritten

Remove `SyncConnected`/`SyncDisconnected` events from `NetworkEvenStream` and provide those events through `ChainSyncInterface` instead. Modify BEEFY/GRANDPA/transactions protocol and `NetworkGossip` to take `SyncEventStream` object which they listen to for incoming sync peer events.

This interface provides a set of miscellaneous functions that other subsystems can use to query, for example, the syncing status.

Subscribe to `NetworkStreamEvent` and poll the incoming notifications and substream events from `SyncingEngine`. The code needs refactoring.

This commits removes the last hard dependency of syncing from `sc-network` meaning the protocol now lives completely outside of `sc-network`, ignoring the hardcoded peerset entry which will be addressed in the future. Code needs a lot of refactoring.

They were written for the sole purpose of verifying that `NetworWorker` continues to function while the calls are being dispatched to `ChainSync`.

…oring

…network

- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon, paritytech/substrate#13535 - Update return values and arguments, b/c syncing protocol was extracted from sc-network. See paritytech/substrate#12828 - Update call to construct_genesis_block(), see paritytech/substrate#13427 - Use of deprecated associated function `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use `from_parts` instead.

* Move import queue out of `sc-network` Add supplementary asynchronous API for the import queue which means it can be run as an independent task and communicated with through the `ImportQueueService`. This commit removes removes block and justification imports from `sc-network` and provides `ChainSync` with a handle to import queue so it can import blocks and justifications. Polling of the import queue is moved complete out of `sc-network` and `sc_consensus::Link` is implemented for `ChainSyncInterfaceHandled` so the import queue can still influence the syncing process. * Move stuff to SyncingEngine * Move `ChainSync` instanation to `SyncingEngine` Some of the tests have to be rewritten * Move peer hashmap to `SyncingEngine` * Let `SyncingEngine` to implement `ChainSyncInterface` * Introduce `SyncStatusProvider` * Move `sync_peer_(connected|disconnected)` to `SyncingEngine` * Implement `SyncEventStream` Remove `SyncConnected`/`SyncDisconnected` events from `NetworkEvenStream` and provide those events through `ChainSyncInterface` instead. Modify BEEFY/GRANDPA/transactions protocol and `NetworkGossip` to take `SyncEventStream` object which they listen to for incoming sync peer events. * Introduce `ChainSyncInterface` This interface provides a set of miscellaneous functions that other subsystems can use to query, for example, the syncing status. * Move event stream polling to `SyncingEngine` Subscribe to `NetworkStreamEvent` and poll the incoming notifications and substream events from `SyncingEngine`. The code needs refactoring. * Make `SyncingEngine` into an asynchronous runner This commits removes the last hard dependency of syncing from `sc-network` meaning the protocol now lives completely outside of `sc-network`, ignoring the hardcoded peerset entry which will be addressed in the future. Code needs a lot of refactoring. * Fix warnings * Code refactoring * Use `SyncingService` for BEEFY * Use `SyncingService` for GRANDPA * Remove call delegation from `NetworkService` * Remove `ChainSyncService` * Remove `ChainSync` service tests They were written for the sole purpose of verifying that `NetworWorker` continues to function while the calls are being dispatched to `ChainSync`. * Refactor code * Refactor code * Update client/finality-grandpa/src/communication/tests.rs Co-authored-by: Anton <anton.kalyaev@gmail.com> * Fix warnings * Apply review comments * Fix docs * Fix test * cargo-fmt * Update client/network/sync/src/engine.rs Co-authored-by: Anton <anton.kalyaev@gmail.com> * Update client/network/sync/src/engine.rs Co-authored-by: Anton <anton.kalyaev@gmail.com> * Add missing docs * Refactor code --------- Co-authored-by: Anton <anton.kalyaev@gmail.com>

- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon, paritytech/substrate#13535 - Update return values and arguments, b/c syncing protocol was extracted from sc-network. See paritytech/substrate#12828 - Update call to construct_genesis_block(), see paritytech/substrate#13427 - Use of deprecated associated function `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use `from_parts` instead.

* Remove `account` * Update and companions * Companion of paritytech/cumulus#2164 * Companion of paritytech/substrate#13159 * Companion of paritytech/cumulus#1909 * Fmt * Companion of paritytech/polkadot#6744 * Companion of paritytech/cumulus#2245 * Companion of paritytech/substrate#12828 * Companion of paritytech/cumulus#2287 * Companion of paritytech/substrate#13592 * Companion of paritytech/cumulus#2308 * Companion of paritytech/substrate#13410 * Companion of paritytech/substrate#13305 * Companion of polkadot-evm/frontier#1050 * TODO weight * TODO weight * Companion of polkadot-evm/frontier#1040 Signed-off-by: Xavier Lau <xavier@inv.cafe> * Remove unused dep * Try fix dev node paritytech/substrate#12828 * Fix the frontier part (#1154) * Fix dev node * Fmt * Bump moonbeam * Bump moonbeam * Remove unnecessary clone * Fix tests --------- Signed-off-by: Xavier Lau <xavier@inv.cafe> Co-authored-by: Guantong <i@gt.email> Co-authored-by: bear <boundless.forest@outlook.com>

Polkadot-Forum · 2023-05-24T07:31:33Z

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-41-v0-9-42/2828/3

- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon, paritytech/substrate#13535 - Update return values and arguments, b/c syncing protocol was extracted from sc-network. See paritytech/substrate#12828 - Update call to construct_genesis_block(), see paritytech/substrate#13427 - Use of deprecated associated function `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use `from_parts` instead.

* Companion for paritytech/substrate#12764 * Remove `async-trait` * Companion for paritytech/substrate#12828 * carg fmt * Update client/relay-chain-minimal-node/src/network.rs Co-authored-by: Bastian Köcher <git@kchr.de> * update lockfile for {"polkadot", "substrate"} --------- Co-authored-by: Bastian Köcher <git@kchr.de> Co-authored-by: parity-processbot <>

* Move import queue out of `sc-network` Add supplementary asynchronous API for the import queue which means it can be run as an independent task and communicated with through the `ImportQueueService`. This commit removes removes block and justification imports from `sc-network` and provides `ChainSync` with a handle to import queue so it can import blocks and justifications. Polling of the import queue is moved complete out of `sc-network` and `sc_consensus::Link` is implemented for `ChainSyncInterfaceHandled` so the import queue can still influence the syncing process. * Move stuff to SyncingEngine * Move `ChainSync` instanation to `SyncingEngine` Some of the tests have to be rewritten * Move peer hashmap to `SyncingEngine` * Let `SyncingEngine` to implement `ChainSyncInterface` * Introduce `SyncStatusProvider` * Move `sync_peer_(connected|disconnected)` to `SyncingEngine` * Implement `SyncEventStream` Remove `SyncConnected`/`SyncDisconnected` events from `NetworkEvenStream` and provide those events through `ChainSyncInterface` instead. Modify BEEFY/GRANDPA/transactions protocol and `NetworkGossip` to take `SyncEventStream` object which they listen to for incoming sync peer events. * Introduce `ChainSyncInterface` This interface provides a set of miscellaneous functions that other subsystems can use to query, for example, the syncing status. * Move event stream polling to `SyncingEngine` Subscribe to `NetworkStreamEvent` and poll the incoming notifications and substream events from `SyncingEngine`. The code needs refactoring. * Make `SyncingEngine` into an asynchronous runner This commits removes the last hard dependency of syncing from `sc-network` meaning the protocol now lives completely outside of `sc-network`, ignoring the hardcoded peerset entry which will be addressed in the future. Code needs a lot of refactoring. * Fix warnings * Code refactoring * Use `SyncingService` for BEEFY * Use `SyncingService` for GRANDPA * Remove call delegation from `NetworkService` * Remove `ChainSyncService` * Remove `ChainSync` service tests They were written for the sole purpose of verifying that `NetworWorker` continues to function while the calls are being dispatched to `ChainSync`. * Refactor code * Refactor code * Update client/finality-grandpa/src/communication/tests.rs Co-authored-by: Anton <anton.kalyaev@gmail.com> * Fix warnings * Apply review comments * Fix docs * Fix test * cargo-fmt * Update client/network/sync/src/engine.rs Co-authored-by: Anton <anton.kalyaev@gmail.com> * Update client/network/sync/src/engine.rs Co-authored-by: Anton <anton.kalyaev@gmail.com> * Add missing docs * Refactor code --------- Co-authored-by: Anton <anton.kalyaev@gmail.com>

altonen added 22 commits November 23, 2022 09:46

Move stuff to SyncingEngine

1d1b955

Move ChainSync instanation to SyncingEngine

bd8f6a2

Some of the tests have to be rewritten

Move peer hashmap to SyncingEngine

9569593

Let SyncingEngine to implement ChainSyncInterface

befeac3

Introduce SyncStatusProvider

c642a33

Move sync_peer_(connected|disconnected) to SyncingEngine

badfbf3

Introduce ChainSyncInterface

0b11339

This interface provides a set of miscellaneous functions that other subsystems can use to query, for example, the syncing status.

Move event stream polling to SyncingEngine

6f4ac98

Subscribe to `NetworkStreamEvent` and poll the incoming notifications and substream events from `SyncingEngine`. The code needs refactoring.

Fix warnings

caf54b4

Code refactoring

a920a7e

Use SyncingService for BEEFY

5dd14e3

Use SyncingService for GRANDPA

83509c9

Remove call delegation from NetworkService

bb005b6

Remove ChainSyncService

e5d6c49

Remove ChainSync service tests

a4f5403

They were written for the sole purpose of verifying that `NetworWorker` continues to function while the calls are being dispatched to `ChainSync`.

Merge remote-tracking branch 'origin/master' into import-queue-refact…

f485d89

…oring

Refactor code

2acb775

Merge branch 'import-queue-refactoring' into extract-syncing-from-sc-…

b01586a

…network

Refactor code

0085220

altonen requested review from acatangiu and andresilva as code owners December 2, 2022 09:55

altonen requested review from bkchr, dmitry-markin and a team December 2, 2022 09:55

github-actions bot added the A0-please_review Pull request needs code review. label Dec 2, 2022

altonen added B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Dec 2, 2022

kiltbot mentioned this pull request Apr 3, 2023

3️⃣ [AUTOMATIC] Update Polkadot dependencies from 0.9.39 to 0.9.40 KILTprotocol/kilt-node#496

Closed

librelois mentioned this pull request Apr 6, 2023

Update substrate/polkadot/cumulus from v0.9.38 to v0.9.40 moonbeam-foundation/moonbeam#2200

Closed

nbaztec mentioned this pull request Apr 25, 2023

Dependency (Substrate/Polkadot/Frontier/Cumulus/...) update to v0.9.40 moonbeam-foundation/moonbeam#2202

Merged

AurevoirXavier added a commit to darwinia-network/darwinia that referenced this pull request May 22, 2023

Companion of paritytech/substrate#12828

e45bca6

jiguantong added a commit to darwinia-network/darwinia that referenced this pull request May 23, 2023

Try fix dev node paritytech/substrate#12828

f80e7b9

b-yap mentioned this pull request May 25, 2023

upgrade to polkadot-v0.9.40 pendulum-chain/spacewalk#350

Merged

ashutoshvarma mentioned this pull request May 31, 2023

Update to polkadot-v0.9.40 AstarNetwork/Astar#948

Merged

6 tasks

kacperzuk-neti mentioned this pull request Jun 23, 2023

Polkadot v0.9.43 liberland/liberland_substrate#295

Merged

15 tasks

salman01zp mentioned this pull request Jul 12, 2023

Update polkadot deps to latest version v0.9.43 tangle-network/dkg-substrate#675

Merged

dmitrylavrenov mentioned this pull request Nov 28, 2023

Tracking issue to bump substrate related deps to polkadot-v0.9.40 humanode-network/humanode#839

Closed

46 tasks

surangap mentioned this pull request May 29, 2024

Upgrade Substrate to v1.0.0 futureversecom/trn-seed#821

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract syncing protocol from `sc-network` #12828

Extract syncing protocol from `sc-network` #12828

altonen commented Dec 2, 2022 •

edited

Loading

Polkadot-Forum commented May 24, 2023

Extract syncing protocol from sc-network #12828

Extract syncing protocol from sc-network #12828

Conversation

altonen commented Dec 2, 2022 • edited Loading

Polkadot-Forum commented May 24, 2023

Extract syncing protocol from `sc-network` #12828

Extract syncing protocol from `sc-network` #12828

altonen commented Dec 2, 2022 •

edited

Loading